Skip to content

Fix the any_scan.h value-param findings caused by #272 - #278

Merged
helly25 merged 2 commits into
mainfrom
clang_tidy_any_scan
Aug 9, 2026
Merged

Fix the any_scan.h value-param findings caused by #272#278
helly25 merged 2 commits into
mainfrom
clang_tidy_any_scan

Conversation

@helly25

@helly25 helly25 commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Clears all 45 performance-unnecessary-value-param findings in mbo/container/any_scan.h — 10% of everything remaining.

This is a regression I introduced in #272

#272 changed AnyScanImpl's constructors to take MakeAnyScanData by const reference. That silently broke the three public wrappers — AnyScan, ConstScan, ConvertingScan — which take the same type by value and then std::move(data) into AnyScanImpl:

AnyScan(container_internal::MakeAnyScanData<Container, ScanMode::kAny> data)
    : AnyScanImpl(std::move(data)) {}   // moving into a const& -- a no-op

Moving into a const& parameter does nothing, so those std::move calls became misleading dead code and the by-value parameters became "copied for each invocation but only used as a const reference".

The count went 25 → 45: the findings moved off the impl constructor and onto the three wrappers, and multiplied. My verification for #272 confirmed the specific diagnostic cleared but never re-measured the file, so this went unnoticed until the enforcement sweep.

Fix

Make the three wrappers take a const reference too, and drop the pointless std::move. Consistent with the type: 16 bytes holding one const std::shared_ptr, whose const member makes even a move a copy (measured in #272).

Test

  • performance-unnecessary-value-param reports zero for the header (was 45).
  • bazel test --config=clang //...109/109 pass.
  • pre-commit run -a green.

Remaining in this header, for later PRs: readability-redundant-typename (5), cppcoreguidelines-rvalue-reference-param-not-moved (3), and single findings from three other checks.

helly25 and others added 2 commits August 8, 2026 23:48
#272 changed AnyScanImpl's constructors to take MakeAnyScanData by const
reference. That silently broke the three public wrappers - AnyScan,
ConstScan and ConvertingScan - which take the same type BY VALUE and then
`std::move(data)` into AnyScanImpl. Moving into a const& parameter is a
no-op, so those moves became misleading dead code and the by-value
parameters became "copied but only used as a const reference".

The finding count went from 25 (at the impl constructor) to 45 (at the
three wrappers): my verification for #272 confirmed the specific
diagnostic cleared without re-measuring the file, so the regression went
unnoticed.

Make the three wrappers take a const reference too and drop the pointless
`std::move`. That is consistent with the type: 16 bytes holding one const
shared_ptr, whose const member makes even a move a copy.

performance-unnecessary-value-param now reports zero for the header.
bazel test --config=clang //... - 109/109 pass.

Signed-off-by: helly25 <6420169+helly25@users.noreply.github.com>
@helly25
helly25 enabled auto-merge (squash) August 9, 2026 08:29
@helly25
helly25 requested a review from Fab-Cat August 9, 2026 08:29
@helly25
helly25 merged commit e9a7c4c into main Aug 9, 2026
23 checks passed
@helly25
helly25 deleted the clang_tidy_any_scan branch August 9, 2026 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants